Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Warns #50

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Warns #50

wants to merge 2 commits into from

Conversation

stsp
Copy link
Contributor

@stsp stsp commented Apr 13, 2024

Warning fixes that are still relevant.

/home/stas/src/SmallerC/v0100/smlrc.c: In function 'IncludeFile':
/home/stas/src/SmallerC/v0100/smlrc.c:1160:11: warning: 'strcpy' offset [-2147483647, -2] is out of the bounds [0, 768] of object 'FileNames' with type 'char[8][96]' [-Warray-bounds=]
 1160 |           strcpy(FileNames[FileCnt] + plen + 1, TokenValueString);
      |           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/stas/src/SmallerC/v0100/smlrc.c:586:6: note: 'FileNames' declared here
  586 | char FileNames[MAX_INCLUDES][MAX_FILE_NAME_LEN + 1];
      |      ^~~~~~~~~

strlen() returns size_t so gcc assumes int can overflow.
/home/stas/src/SmallerC/v0100/smlrc.c: In function 'exprval':
/home/stas/src/SmallerC/v0100/smlrc.c:3934:23: warning: array subscript -1 is below array bounds of 'unsigned char[3072]' [-Warray-bounds=]
 3934 |       if (SyntaxStack0[*ExprTypeSynPtr] == tokStructPtr && !GetDeclSize(*ExprTypeSynPtr, 0))
      |           ~~~~~~~~~~~~^~~~~~~~~~~~~~~~~
/home/stas/src/SmallerC/v0100/smlrc.c:679:15: note: while referencing 'SyntaxStack0'
  679 | unsigned char SyntaxStack0[SYNTAX_STACK_MAX];
      |               ^~~~~~~~~~~~

Make it clear with assert() that *ExprTypeSynPtr is not negative here.
@stsp
Copy link
Contributor Author

stsp commented Apr 13, 2024

You applied the smlrl fixes but smlrc
still gives warnings. Interestingly
smlrcc no longer gives warnings, even
though no fixes were applied there.
I guess gcc improved its detection logic.

@alexfru
Copy link
Owner

alexfru commented Apr 13, 2024

I didn't do anything about these because my version of gcc doesn't warn here (yet). Fixing warnings in the dark is kinda silly, especially so if I need a fix different from the proposed.

In smlrcc I did cap the file name length (arbitrarily at 255 chars) and that was enough for gcc to see name length + file length fit into 10 decimal digits (the file length had already been capped). There was also some fixing around realloc() in fatargs(), which is used in smlrl, smlrcc, smlrpp, but those are probably unrelated.

@stsp
Copy link
Contributor Author

stsp commented Apr 13, 2024

Fixing warnings in the dark is kinda silly, especially so if I need a fix different from the proposed.

So why not to apply my patch then? :)

In smlrcc I did cap the file name length (arbitrarily at 255 chars)

Ah, good to know.
I was wondering where those warning went.

@alexfru
Copy link
Owner

alexfru commented Apr 13, 2024

Fixing warnings in the dark is kinda silly, especially so if I need a fix different from the proposed.

So why not to apply my patch then? :)

I wouldn't know if it was fixed and gone.

@stsp
Copy link
Contributor Author

stsp commented Apr 13, 2024

Don't you believe me that they are
gone? The remained patches are trivial.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants